Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Help older Python parse new Git dates #3721

Merged
merged 1 commit into from
May 19, 2024

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented May 19, 2024

Current Git (2.45.1) produces strict ISO dates with Z for any time with +00:00 time zone offest. Python prior to 3.11 does not understand these dates. This trunkes the Z in case of ValueError which is what old Python throws. The +00:00 offset is placed in the input string for fromisoformat instead.

>
Current Git (2.45.1) produces strict ISO dates with Z for any time with +00:00 time zone offest. Python prior to 3.11 does not understand these dates. This trunkes the Z in case of ValueError which is what old Python throws. The +00:00 offset is placed in the input string for fromisoformat instead.
@github-actions github-actions bot added the Python Related code is in Python label May 19, 2024
@wenzeslaus wenzeslaus changed the title build: Help older Python parse new Git dates > Current Git (2.45.1) produces strict ISO dates with Z for any time with +00:00 time zone offest. Python prior to 3.11 does not understand these dates. This trunkes the Z in case of ValueError which is what old Python throws. The +00:00 offset is placed in the input string for fromisoformat instead. build: Help older Python parse new Git dates May 19, 2024
@wenzeslaus
Copy link
Member Author

wenzeslaus commented May 19, 2024

At least one of the GCC runs here is using the new image, 20240516.1, and it runs, so I'm assuming the change helped.

Runner Image
  Image: ubuntu-22.04
  Version: 20240516.1.0
  Included Software: https://github.com/actions/runner-images/blob/ubuntu22/20240516.1/images/ubuntu/Ubuntu2204-Readme.md

Anyway, not my favorite change. This should be removed again when our min Python version is 3.11.

@echoix
Copy link
Member

echoix commented May 19, 2024

What if we go another way, and ask Git to output the specific format we want, that would work for any locale settings?

I know that for multiple git commands, I already set the format I wanted, it wasn't git log, but another less common command and the usage was similar

@wenzeslaus
Copy link
Member Author

ask Git to output the specific format we want

That's what we tried to do with requesting strict ISO.

With other things I would perhaps say yes, specifying custom format is reasonable, but going into the whole date formatting, I don't know...

What will we do when we drop Python 3.10 support? Go back to the code which is now on main or keep maintaining our own date format just for this piece of code? I say, let's work towards using ISO and this PR does that. I think it follows the trend in Python and Git: Python supported subset of ISO for parsing, now they support the whole thing. Git had some ISO-like format they called ISO and then then introduced strict ISO. Now they made change they believe is closer to the standard (I'm not convinced about that, but that's not the point here).

@wenzeslaus wenzeslaus added CI Continuous integration bug Something isn't working labels May 19, 2024
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing the way that this fix is implemented, and verifying that no other time zones end with Z (from lists of time zones wiki pages), I think this fix is reasonable. It tries the happy path, then tries one fix for a common failure, and then fails.

@echoix echoix added this to the 8.4.0 milestone May 19, 2024
@echoix echoix merged commit 85bf53b into OSGeo:main May 19, 2024
27 checks passed
@echoix
Copy link
Member

echoix commented May 19, 2024

Is this a candidate for backporting? (On what branches the git dates were used?)

@echoix echoix linked an issue May 19, 2024 that may be closed by this pull request
@echoix
Copy link
Member

echoix commented May 19, 2024

Now merged, multiple builds with Ubuntu 22.04 image 20240516.1 are successful, including ones using Python 3.8 and 3.10.

@wenzeslaus wenzeslaus deleted the support-new-git-with-old-python branch May 21, 2024 00:51
@wenzeslaus
Copy link
Member Author

Is this a candidate for backporting? (On what branches the git dates were used?)

No, we don't plan further releases in the current branches. The only planned release in the Milestones is 8.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI Continuous integration Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests are failing in CI with an ISO format error
2 participants